Type configs #97

dannyroberts · 2014-09-08T19:00:43Z

This is a modest restructuring that makes all of a JsonObject subclass's type inferences configurable. It's done like this

class MyJsonObject(JsonObject):
    class Meta(object):
        # this replaces the default DateTimeProperty with your custom one
        # you can also completely override all properties from scratch with
        # properties = {...}
        update_properties = {datetime.datetime: MyBetterDateTimeProperty}

Now, in a MyJsonObject subclass, you'll still be able to have DateTimePropertys, but in the case of a dynamic property (one that you didn't specify in the schema) it will now default to using MyBetterDateTimeProperty.

test that the reason it was added does not break

- reverts/replaces the recent "_string_conversions" addition to the api - adds a 'Meta' class to the JsonObject api, for configuring this kind of setting - move "convert" file's parts into other files

this is to allow, for example, couchdbkit to highjack this as well

millerdev · 2014-09-09T12:13:31Z

jsonobject/api.py

+re_datetime = re.compile(
+    r'^(\d{4})\D?(0[1-9]|1[0-2])\D?([12]\d|0[1-9]|3[01])'
+    '(\D?([01]\d|2[0-3])\D?([0-5]\d)\D?([0-5]\d)?\D?(\d{3,6})?'
+    '([zZ]|([\+-])([01]\d|2[0-3])\D?([0-5]\d)?)?)?$'


Not sure it matters in this case, but you need to put an r in front of each line if you want each line to be that type of string literal:

>>> (r"abc\n" ... "\ncba") 'abc\\n\ncba' >>> (r"abc\n" ... r"\ncba") 'abc\\n\\ncba'

Wow, great catch. You're right that it doesn't "matter" in this case, because two python "oddities" cancel each other out: (1) string-type prefixes only apply to the first in a multiline concatenation (ok I mean that makes sense) (2) "\d" is silently treated as two characters "\d" for any character that isn't part of an escape sequence, while "\n" is a single character. So the loosey-gooseyness of (2) obscured (1) and hasn't been causing an error.

millerdev · 2014-09-09T13:34:20Z

setup.py

@@ -5,7 +5,7 @@

 setup(
    name='jsonobject',
-    version='0.5.0',
+    version='0.6.0',


Yesterday you had mentioned that you need to push this up to PyPI to test it. If you're still thinking of doing that, then what about changing the version to 0.6.0b1 or 0.6.0c1?

Sounds good.

millerdev · 2014-09-09T20:17:59Z

jsonobject/base.py

-        # There's a pretty fundamental cyclic dependency between this metaclass
-        # and knowledge of all available property types (in properties module).
-        # The current solution is to monkey patch this metaclass
-        # with a reference to the properties module


Really glad to see this comment removed. Was hoping it was no longer relevant

Yeah it was actually one very related to what I was doing, so this naturally got fixed in the process.

Type configs

millerdev · 2014-09-10T13:52:24Z

jsonobject/base.py

+
+        class Foo(JsonObject):
+            # property definitions go here
+            # ...


Was thinking about this last night and wondering if it might make sense to put a timestamp = ... property definition here? The part below where you say "If you now do ... foo.timestamp = ..." seems pretty magic since there is no connection between foo.timestamp (indeed, it's not even defined) and MySpecialDateTimeProperty. The question I'm left with is what is making that connection? How does Foo.timestamp need to be defined to be affected by that Meta block?

Aha that's actually the whole point though, that you don't have to define timestamp as a property. By adding datetime.datetime: MySpecialDateTimeProperty to Meta, you're declaring that all dynamic properties that are set to a datetime should be governed by MySpecialDateTimeProperty instead of by the default.

Wow, that's some pretty deep magic there. I'm guessing that MySpecialDateTimeProperty knows how to serialize a date as a string when the object is serialized as JSON, right? How does it know which strings it should deserialize when the object is loaded from persistent storage?

I like how this is coming out in layers. Yes, you're totally right that there needs to be some mechanism for deciding whether a dynamic string should be treated as a string or as some other type. This is done by setting string_conversions in Meta, which is basically a mapping from regular expressions to data types; if you want to see what that looks like, check the default used in the JsonObject class definition (within this PR). Keep asking questions!

Ah, thank you, string_conversions was a critical connection I had not made yet. Would it make sense to add string_conversions to the Meta class below to provide a more complete example that demonstrates all the pieces needed for a successful persist/load round-trip?

What if someone happens to put a string that successfully passes through a string conversion into a field that is supposed to remain a string?

Yeah so that's one of those things that is carried over from couchdbkit: if you wrap JSON that has properties not listed in the schema, it automatically converts strings in particular formats to the their respective types. This PR takes a step towards loosening that hardcoded behavior, so that if you want, for example, you can set

class Meta(object): string_conversions = ()

and then no dynamic string conversions happen.

One possible approach to avoid accidental reverse conversion of strings would be to store a list of the fields that were dynamically converted along with a hint about the reverse conversion procedure. Then when the object is loaded from the db that information can be used to do the reverse conversion. Maybe that's too much overhead though? (although given that it's running every string through a sequence of regexes on load...)

I see what you're saying, but I think that that's too much of a departure from the current API to implement now without a fair amount of work in the calling code. It's also have to add some extra property like _dynamic_properties to the JSON output that the caller didn't explicitly ask for.

dannyroberts added 7 commits September 8, 2014 11:39

get rid of redundant ALLOWED_PROPERTY_TYPES

96e6651

remove basestring from MAP_TYPES_PROPERTIES

ad5f153

test that the reason it was added does not break

make the treatment of types completely configurable

d28c0d2

- reverts/replaces the recent "_string_conversions" addition to the api - adds a 'Meta' class to the JsonObject api, for configuring this kind of setting - move "convert" file's parts into other files

add and test update_properties Meta variable

55f2fc5

remove support for python 2.6

e82deb3

allow unknown options in Meta

88535ec

this is to allow, for example, couchdbkit to highjack this as well

bump version to 0.6.0

59a46c4

dannyroberts mentioned this pull request Sep 8, 2014

make changes to accomodate jsonobject type config changes dimagi/couchdbkit#3

Merged

czue assigned millerdev Sep 8, 2014

millerdev reviewed Sep 9, 2014
View reviewed changes

r-prefix all regexes

efa2924

millerdev reviewed Sep 9, 2014
View reviewed changes

dannyroberts added 5 commits September 9, 2014 11:23

document TypeConfig class

f1f4973

change version to beta

7c49c72

remove stale comment

b7c0d60

better assert

77e0af3

change TypeConfig.update to updated

64d3a4d

millerdev reviewed Sep 9, 2014
View reviewed changes

millerdev added a commit that referenced this pull request Sep 9, 2014

Merge pull request #97 from dimagi/type-configs

c736667

Type configs

millerdev merged commit c736667 into master Sep 9, 2014

millerdev deleted the type-configs branch September 9, 2014 20:19

millerdev reviewed Sep 10, 2014
View reviewed changes

dannyroberts mentioned this pull request Sep 24, 2014

Save timezone dimagi/commcare-hq#4386

Closed

9 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Type configs #97

Type configs #97

dannyroberts commented Sep 8, 2014

millerdev Sep 9, 2014

dannyroberts Sep 9, 2014

millerdev Sep 9, 2014

dannyroberts Sep 9, 2014

millerdev Sep 9, 2014

dannyroberts Sep 9, 2014

millerdev Sep 10, 2014

dannyroberts Sep 10, 2014

millerdev Sep 11, 2014

dannyroberts Sep 11, 2014

millerdev Sep 11, 2014

dannyroberts Sep 11, 2014

dannyroberts Sep 11, 2014

millerdev Sep 11, 2014

dannyroberts Sep 12, 2014

Type configs #97

Type configs #97

Conversation

dannyroberts commented Sep 8, 2014

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment